Bug#102586: multi-table DELETE with ON DELETE CASCADE breaks row-based replication by matanbaruch · Pull Request #719 · mysql/mysql-server

matanbaruch · 2026-08-13T09:55:40Z

What does this change do?

A multi-table DELETE that names both a foreign key parent table and its ON DELETE CASCADE child breaks row-based replication: the replica applier stops with ER_KEY_NOT_FOUND. This defers the delete until the join has finished when a cascade dependency exists between tables inside the query, on both the classic and the hypergraph optimizer path.

BUG#102586, BUG#80821

Why is it needed?

Both bugs are Verified and still unfixed. #80821 was reported in 2016, #102586 in 2021, and neither has moved since October 2023. #102586 is S1.

What happens:

The parent row is deleted while the join is still scanning.
The cascade removes the child rows and logs row events for them.
The statement also logs row events for the child rows it deletes itself.
On the replica the parent delete is applied first, its own cascade removes the child rows, and the logged child events cannot find them. Error 1032.

A single-table DELETE that relies on the cascade is fine. Only the multi-table form is affected.

We hit this in production on 8.4.8 LTS, which is listed on neither bug report. Three replicas broke inside the same five minute window and stayed broken for three weeks, because Amazon RDS kept restarting the applier and the lag looked normal. A local harness reproduces it identically on 8.0.35, so this is not a regression in a recent version, it has just never been fixed.

The design is Zsolt Parragi's, contributed on #80821 in 2019 and acknowledged by Oracle at the time. That patch no longer applies: get_cascade_foreign_key_table_list() was removed. This version resolves the cascade dependency from TABLE_SHARE::foreign_key_parent instead, and covers the hypergraph optimizer, which did not exist in 2019.

How was it tested?

Added/updated MTR tests under mysql-test/
scripts/ci/mtr.sh passes locally
Ran the relevant full suite (name it): rpl, 1100 tests, --parallel=8 --force

New test: rpl.rpl_multi_table_delete_fk_cascade. Debug build, 26.7.0, Linux, before and after the same commit:

	classic optimizer	hypergraph optimizer
trunk	fail: 1032, `HA_ERR_KEY_NOT_FOUND` on `test.t2`	fail: same
this branch	pass	pass

Both optimizer paths were checked separately because they decide immediate deletion in different places, and trunk fails in both.

Also passing on this branch: main.foreign_key_cascade, innodb.innodb, innodb.innodb_misc1 (the existing tests that combine cascading foreign keys with multi-table DELETE), and the 25 tests matching --do-test=delete.

The test keeps an ON DELETE SET NULL case as negative coverage. That one replicates correctly on trunk, so it is deliberately left in immediate mode: deferring it would change which rows the statement removes.

Two tests failed in the full rpl run and both pass in isolation on the same binary, so I read them as flaky under --parallel=8 rather than caused by this change: rpl_crash_on_pfs_worker_table_against_replica_stop and rpl_parallel_alter_db_table. Neither involves DELETE or foreign keys. If they are known-stable in your CI, say so and I will dig further.

Contributor checklist

I have signed the OCA with the email on these commits
Code is formatted (scripts/ci/format.sh)
Commits are focused with descriptive messages

AI assistance

I did not use AI assistance for this contribution
I used AI assistance for this contribution

Claude Code wrote the patch and the MTR test. The fix design is not new, it is Parragi's 2019 approach from #80821 rebased onto current trunk. Everything claimed above was verified by running it, not by inspection: the before and after matrix comes from real MTR runs on a local Debug build, in both optimizer modes. The production root cause behind it was diagnosed separately against a Docker reproduction of our schema.

Areas touched

optimizer, replication

A multi-table DELETE that names both a foreign key parent table and a child table with a cascading delete rule breaks row-based replication. The replica applier stops with ER_KEY_NOT_FOUND. The parent row is deleted while the join is still scanning, so the cascade removes the child rows and logs row events for them. The statement logs row events for the child rows it deletes itself as well. On the replica the parent delete is applied first, its own cascade removes the child rows, and the logged child events then cannot find them. Exclude a delete target from immediate deletion when deleting from it cascades to another table in the same query, which defers the delete until the join has finished. The check is added to both the classic optimizer (GetImmediateDeleteTables) and the hypergraph optimizer (IsImmediateDeleteCandidate). Only ON DELETE CASCADE is considered. ON DELETE SET NULL updates the child rows rather than deleting them, so they stay findable for the logged events and replicate correctly. Deferring those deletes as well would change which rows the statement removes. This is the approach Zsolt Parragi contributed on Bug#80821 in 2019, adapted to the current code: get_cascade_foreign_key_table_list() no longer exists, so the cascade dependency is resolved from TABLE_SHARE::foreign_key_parent instead.

matanbaruch · 2026-08-13T09:55:49Z

I confirm the code being submitted is offered under the terms of the OCA, and that I am authorized to contribute it

matanbaruch · 2026-08-14T04:13:24Z

The three red checks here are CI-side and all of them reproduce without this patch. Writing up what I found, in case it is useful:

1. Build, MTR and Format Check never see the code. All three workflows trigger on pull_request_target and check out refs/pull/N/merge, which GitHub now refuses for forks:

Refusing to check out fork pull request code from a 'pull_request_target' workflow.

Checkout fails in about 5 seconds and every later step then fails with working directory .../source: No such file or directory. #714 and #716 fail identically. #713 still built for 8m21s, so this arrived with the recent contribution automation change.

2. Format Check fails on trunk itself. It runs clang-format-18 --style=file --dry-run --Werror over whole changed files. Three of the files this PR touches are not clean under clang-format 18 at base commit 06a5c1c99, with nothing applied: sql/sql_base.cc:3185, sql/sql_delete.cc:634, sql/join_optimizer/join_optimizer.cc:4760 and :10034. Those are exactly the lines reported on this PR, shifted by my insertions. Running the formatter over those three files rewrites 2306 lines, including include ordering and signature reflows, which looks like the tree was formatted with a different clang-format version. I left them alone rather than bury a 54 line fix. The lines this PR adds are clean under git clang-format.

3. MTR cannot finish inside the budget. mtr.yml is runs-on: ubuntu-latest with timeout-minutes: 360, and GitHub's hard job limit is 6h. A Debug build plus the default MTR selection reached 25% in 6h on that runner class and was killed, with no test failures. There are no successful MTR runs in this repository's history.

Since CI here cannot run the code, I ran your workflows unmodified on a fork-internal PR, where checkout is allowed:

Debug build (gcc): pass, 1h23m
Debug build (clang): pass, 1h40m
rpl.rpl_multi_table_delete_fk_cascade 'row': pass

Local Debug build of 26.7.0, same commit, before and after:

	classic	hypergraph
trunk	1032, `HA_ERR_KEY_NOT_FOUND` on `test.t2`	same
this branch	pass	pass

Both optimizer paths decide immediate deletion in different places and trunk fails in both, which is why the patch touches both.

If you want the format gate green on this PR I can add a separate commit that reformats those three files, but it is 2306 unrelated lines and I would rather not unless you ask for it.

RidhaOracle · 2026-08-14T16:15:04Z

@matanbaruch You are totally right regarding about the github actions failures. I have described the issue in more details in the issue #715. I have a patch for it that will be deployed early next week, it took a bit of time to deploy it as I also tried to stabilize the MTRs. Sorry about the noise !

Run untrusted pull request builds with restricted permissions against validated revisions, and publish statuses and labels only from trusted workflows that revalidate the repository, workflow run, PR head, and ordering. Replace the custom review client with the pinned OpenAI Codex Action, bound its input to a validated PR diff, pin third-party actions, and add dependency maintenance for GitHub Actions. Retry a failed or empty Codex review once after a delay with a configurable fallback model while preserving the same read-only isolation boundary and structured output contract. Publish structured Codex findings as one commit-bound GitHub review. Validate each file and right-side line range against the current diff, keep unanchored findings in the summary, prevent duplicate reviews, and revalidate both reviewed revisions before posting. Warm trusted Boost and ccache entries, align the MTR compiler cache with the GCC build, shard MTR suites across runners, run tests in parallel with bounded retries, and retain diagnostics. Safely reset head-scoped CI state, standardize labels, and remove the obsolete OCA checkbox. Require both the OCA Verified label and a current trusted approval before adding Integrate. Revalidate both conditions around label publication and remove Integrate if either condition no longer holds. Temporarily disable parallel-run failures tracked by Bug#39882117 and restore the required restart and expected output for the buffer-pool-load MTR. Change-Id: I7393e75cab3afa172a99237337c26e3974f955fa

RidhaOracle · 2026-08-21T08:29:44Z

@matanbaruch The fix for issue #715 have been merged in trunk. Can you please rebase your branch to trigger the pipelines ?

… change Format Check runs clang-format-18 over whole changed files. These three are not clean under 18 on trunk, so the gate fails for any PR touching them. Cosmetic only: a label space in sql_base.cc, one DBUG_LOG argument wrap in sql_delete.cc, and two string literal joins in join_optimizer.cc.

matanbaruch · 2026-08-21T17:42:22Z

@RidhaOracle MTR (replication) failed only on rpl_stm_gtid_disconnect_drop_temporary_table, a REPLICA_OPEN_TEMP_TABLES timing race unrelated to this patch (no DELETE, no FK, and the same commit passed that shard on my fork). Can you re-run the shard?

matanbaruch · 2026-08-25T15:25:14Z

@RidhaOracle can you re-run the MTR (replication) shard and point a reviewer at this? Everything else is green and the only failure is the temp table race, not this patch.

matanbaruch · 2026-08-28T18:00:33Z

Pushed. All four points addressed in 6a37612:

Every referential action except RESTRICT and NO ACTION is now considered. Your SET NULL repro produces the correct result (t1 empty, all t2.t1_id NULL) and is in the test verbatim.
The check now follows referential actions transitively. Whether a child's own children are checked through their delete rule or their update rule depends on whether that child's rows get deleted (CASCADE on a delete path) or updated (SET NULL, SET DEFAULT, or any update-path action). Intermediate tables are found among the open tables, which prelocking guarantees to include everything reachable through referential actions; there is an assert plus a conservative fallback if that ever does not hold.
Multi-table UPDATE is covered in safe_update_on_fly() and IsImmediateUpdateCandidate(). One deliberate simplification: the check does not verify that the updated columns overlap the FK's referenced columns, so an UPDATE that cannot fire the action still gets deferred when a modifying child is in the query. Say the word and I will add the column-overlap refinement.
The rpl test is gone. The new test is main.foreign_key_multi_table_dml, single server, wrong-results based, containing both of your repros plus an indirect case (t1 cascades into t_mid, t_mid into t3, only t1 and t3 in the query) and two multi-table UPDATE cases.

Verified locally on a Debug build: the new test passes with both optimizer modes, and foreign_key (44 tests), delete (25), update (24), innodb.innodb, innodb.innodb_misc1 and foreign_key_cascade all pass.

matanbaruch · 2026-08-29T00:05:56Z

@kahatlen @RidhaOracle can one of you approve the workflow runs for 6a37612? Build and MTR are waiting on approval since the last push.

kahatlen · 2026-08-31T13:19:57Z

I focused this pass on the optimizer-visible behavior and concrete reproductions. Since the new dependency traversal is in sql_base.cc and owned by the Runtime team, I think this should also receive a Runtime-team review.

Review follow-up for Bug#102586 / Bug#80821: - Do not assert when a child table is not among the open tables. With engine-managed referential actions (innodb_native_foreign_keys) prelocking does not add child tables, so the walk cannot continue; buffer the modification in that case, since the engine-internal action poses the same hazard. - Track visited tables per (table, deleted-vs-updated) state. The two states can lead to different descendants, so a table reached along both a delete path and an update path in a diamond-shaped foreign key graph must be walked once per state. Keying on the table alone could skip the state that reaches a queried table and incorrectly allow immediate modification. - Make the multi-table UPDATE test cases update a referenced secondary unique key instead of the clustered primary key. Updating the primary key already made the traditional optimizer buffer the update, so the old shape did not exercise the new check.

matanbaruch · 2026-08-31T17:24:09Z

All three points addressed and pushed in b1f9b04.

The assertion is gone. When the child table is not among the open tables, the check now returns true and the modification is buffered. I reproduced the assertion first with --mysqld=--innodb_native_foreign_keys=TRUE main.foreign_key_multi_table_dml as you described, and the full test file now passes in that mode with the same recorded results.
visited is now keyed on (table, deleted-vs-updated) pairs, so a table reached along both kinds of paths is walked once per state. Your diamond repro is in the test. I reproduced the wrong result (root keeping (2, 1)) on the previous head with the default optimizer, matching your verification on both optimizers, and with the fix it produces the correct empty root in both optimizers.
Both UPDATE cases now use your shape: a referenced secondary UNIQUE key is updated, with ON UPDATE CASCADE in one case and ON UPDATE SET NULL in the other. You are right that the old shape never exercised the check, since updating the clustered primary key already forced buffering in the traditional optimizer. Thanks for verifying fail-without-fix and pass-with-fix on both optimizers for this shape.

One separate observation from recording the diamond case: the default (non-native) foreign key handling leaves a dangling reference in that schema. A plain single-table DELETE FROM root; deletes the z_delete row through ON DELETE CASCADE but leaves common_child row (2, NULL, 1) pointing at it, while --innodb_native_foreign_keys=TRUE correctly cascade-deletes that row too. It reproduces at current HEAD without this patch, and it only happens once query_child (the grandchild with ON UPDATE CASCADE) exists. That is why the diamond test checks only root, so it passes identically in both FK modes. I can file a separate bug report with the minimal repro if useful.

@kahatlen @RidhaOracle the push will need workflow approval again for build and MTR to run on b1f9b04.

matanbaruch · 2026-09-01T13:56:32Z

@kahatlen @RidhaOracle can you re-run the storage and services shards for b1f9b04? The failures are runner flakes, not the patch: MY-011959 buffer pool warnings in innodb.log_file_name_1 and innodb_undo.truncate_recover_02 (neither test touches foreign keys or multi-table DML, both pass locally on this commit), and on services the MTR step passed and only the unit test step failed on the NDB testSecureSocket-t abort plus router sharing timing flakes.

…ble-delete-fk-cascade # Conflicts: # mysql-test/collections/disabled.def

matanbaruch · 2026-09-03T12:19:58Z

@kahatlen @RidhaOracle MTR (core) on da35310 fails in information_schema.i_s_schema_definition_debug with "no known I_S version with the given checksum", and it fails identically on unmodified trunk e174239 (our merge base), so trunk is missing a checksum row for I_S version 261000 rather than anything in this patch. All other shards and builds are green.

kahatlen

Thanks for addressing the earlier feedback and strengthening the tests. I found one remaining wrong-results case involving a child table read by a subquery; I’ve added a small reproduction inline. I also left a question about allocation handling for the Runtime reviewer. Thanks for your continued work on this fix!

kahatlen · 2026-09-11T12:23:05Z

+
+      // A modified child that the query reads makes immediate modification
+      // of the subject table unsafe.
+      for (const Table_ref *tl = query_block->leaf_tables; tl != nullptr;


Could this check also account for tables read by subqueries? query_block->leaf_tables only covers the current query block, so a child read by a correlated subquery is missed and the parent can still be deleted immediately.

Unlike unique_table(), which follows next_global and can find references in other query blocks, this loop follows next_leaf and therefore misses the subquery’s reference to child table c.

This example produces a wrong result with both the traditional optimizer and the hypergraph optimizer:

CREATE TABLE p(id INT PRIMARY KEY, i INT); INSERT INTO p VALUES (1, 2), (2, 1); CREATE TABLE c( p_id INT PRIMARY KEY, FOREIGN KEY (p_id) REFERENCES p(id) ON DELETE CASCADE ); INSERT INTO c VALUES (1), (2); DELETE p FROM p WHERE (SELECT COUNT(*) FROM c WHERE c.p_id = p.i) > 0; SELECT * FROM p ORDER BY id; SELECT * FROM c ORDER BY p_id; DROP TABLE c, p;

Expected: both SELECTs return empty results. Actual: p contains (2, 1) and c contains (2).

Both parent rows initially satisfy the predicate. Deleting parent 1 immediately cascades to child 1, so the subquery for parent 2 no longer finds its match and parent 2 is incorrectly skipped. Could the dependency check include reads in other query blocks, with this case added to the regression test?

kahatlen · 2026-09-11T13:02:15Z

+  // two rules can lead to different descendants, a table reached both ways
+  // must be walked once per state, so visited entries are (table, state)
+  // pairs rather than tables.
+  std::vector<std::pair<const TABLE_SHARE *, bool>> pending;


Question for the Runtime reviewer: would Prealloced_array be more appropriate for these traversal lists? In particular, do we need MySQL memory instrumentation and explicit allocation-failure handling here? The current std::vector insertions can throw on allocation failure. I’ll defer to Runtime on the appropriate container and error-handling approach.

Review follow-up for Bug#102586 / Bug#80821: The check for whether a referential action modifies a table read by the statement only scanned the leaf tables of the DELETE or UPDATE query block, so a child table read by a subquery in another query block was missed and the subject table could still be modified immediately, giving wrong results once the action rewrote rows the subquery had not read yet. Walk the complete table list of the statement instead, so reads in all query blocks are seen, stopping before the tables added by prelocking, since those are not read by the statement itself.

matanbaruch · 2026-09-12T17:37:07Z

Fixed in 25600d8. The check now walks the complete table list of the statement instead of the query block's leaf tables, stopping before the tables added by prelocking, so reads in subqueries and other query blocks are seen the same way unique_table() sees them. Your repro is in the test verbatim: I reproduced the wrong result (p keeping (2, 1)) on the previous head first, and with the fix both p and c end up empty, in both optimizers plus the innodb_native_foreign_keys mode. On the Prealloced_array question I left the containers as std::vector for now and will switch to whatever the Runtime reviewer prefers.

@kahatlen @RidhaOracle the new push will need workflow approval again for 25600d8.

github-actions

One test-coverage recommendation: add a row-based replication regression deleting both parent and child, to cover overlapping explicit and cascaded deletes. No high-confidence production-code defect found in the accessible seven-file diff. Review was static: PR metadata was restricted, code search was rate-limited, and no checkout/build was available to run tests.

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

ab.chatgpt.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "ab.chatgpt.com"

See Network Configuration for more information.

Generated by Codex PR Review for #719 · codex · gpt60 · 93.7 AIC · ⌖ 23.4 AIC · ⊞ 13.3K
_{Comment /codex to run again}

github-actions · 2026-09-12T17:43:11Z

+
+# Every row in t1 matches a row in t2, so all rows in t1 should be deleted,
+# and the cascade should keep only the t2 rows that reference no t1 row.
+DELETE t1 FROM t1, t2 WHERE t1.i = t2.id;


Add a row-based replication regression that explicitly deletes both parent and child, for example DELETE t1, t2 FROM t1, t2 WHERE t1.i = t2.id. Every new DELETE case targets only the parent, and this test never replays the binlog on a replica. These assertions therefore do not exercise the interaction between explicit child deletes and cascaded deletes that the replication fix needs to handle. A case in the replication suite should use binlog_format=ROW, synchronize the replica after the DELETE, and compare both tables on source and replica.

…ble-delete-fk-cascade

matanbaruch requested a review from a team August 13, 2026 09:55

matanbaruch requested review from gopshank and seemasundara as code owners August 13, 2026 09:55

oracle-contributor-agreement Bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Aug 13, 2026

github-actions Bot added Review Requested Review requested from code owners Optimizer Changes touching optimizer code Tests Changes touching test code or test data MTR Failed MTR suite failed Build Failed PR build failed labels Aug 13, 2026

matanbaruch mentioned this pull request Aug 13, 2026

CI validation only: Bug#102586 multi-table DELETE FK cascade (do not merge) matanbaruch/mysql-server#1

Closed

seemasundara requested a review from ogrovlen August 14, 2026 16:54

ogrovlen requested a review from kahatlen August 21, 2026 05:32

Merge branch 'trunk' into bug102586-multi-table-delete-fk-cascade

baeaf45

github-actions Bot removed Build Failed PR build failed MTR Failed MTR suite failed labels Aug 21, 2026

github-actions Bot added Build Passed PR build passed MTR Failed MTR suite failed labels Aug 21, 2026

gopshank removed request for gopshank and seemasundara August 22, 2026 03:58

github-actions Bot added MTR Passed MTR suite passed and removed MTR Failed MTR suite failed labels Aug 26, 2026

kahatlen requested a review from prthirum August 31, 2026 13:10

kahatlen reviewed Aug 31, 2026

View reviewed changes

Comment thread sql/sql_base.cc Outdated

Comment thread sql/sql_base.cc Outdated

Comment thread mysql-test/t/foreign_key_multi_table_dml.test Outdated

github-actions Bot added the Build Passed PR build passed label Aug 31, 2026

github-actions Bot removed the Build Passed PR build passed label Aug 31, 2026

github-actions Bot added Build Passed PR build passed MTR Failed MTR suite failed labels Sep 1, 2026

bjornmu force-pushed the trunk branch from 23e360a to e174239 Compare September 2, 2026 11:53

github-actions Bot removed the MTR Failed MTR suite failed label Sep 2, 2026

Merge remote-tracking branch 'upstream/trunk' into bug102586-multi-ta…

da35310

…ble-delete-fk-cascade # Conflicts: # mysql-test/collections/disabled.def

github-actions Bot added Build Passed PR build passed MTR Failed MTR suite failed and removed Build Passed PR build passed labels Sep 3, 2026

henrikingo-oracle mentioned this pull request Sep 3, 2026

MTR failing on trunk after monthly push (information schema) #742

Open

kahatlen reviewed Sep 11, 2026

View reviewed changes

gopshank requested a review from phulakun September 11, 2026 18:09

github-actions Bot removed Build Passed PR build passed MTR Failed MTR suite failed labels Sep 12, 2026

github-actions Bot reviewed Sep 12, 2026

View reviewed changes

Merge remote-tracking branch 'upstream/trunk' into bug102586-multi-ta…

facf677

…ble-delete-fk-cascade

github-actions Bot mentioned this pull request Sep 12, 2026

[aw] Codex PR Review exceeded daily AI credits budget #752

Open

github-actions Bot added the Build Passed PR build passed label Sep 12, 2026

Uh oh!

Conversation

matanbaruch commented Aug 13, 2026

What does this change do?

Why is it needed?

How was it tested?

Contributor checklist

AI assistance

Areas touched

Uh oh!

matanbaruch commented Aug 13, 2026

Uh oh!

matanbaruch commented Aug 14, 2026

Uh oh!

RidhaOracle commented Aug 14, 2026

Uh oh!

RidhaOracle commented Aug 21, 2026

Uh oh!

matanbaruch commented Aug 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

matanbaruch commented Aug 25, 2026

Uh oh!

matanbaruch commented Aug 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

matanbaruch commented Aug 29, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kahatlen commented Aug 31, 2026

Uh oh!

matanbaruch commented Aug 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

matanbaruch commented Sep 1, 2026

Uh oh!

matanbaruch commented Sep 3, 2026

Uh oh!

kahatlen left a comment

Choose a reason for hiding this comment

Uh oh!

kahatlen Sep 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kahatlen Sep 11, 2026

Choose a reason for hiding this comment

Uh oh!

matanbaruch commented Sep 12, 2026

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Sep 12, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

matanbaruch commented Aug 21, 2026 •

edited

Loading

matanbaruch commented Aug 28, 2026 •

edited

Loading

matanbaruch commented Aug 31, 2026 •

edited

Loading

kahatlen Sep 11, 2026 •

edited

Loading